Skip to content

feat: Enable BVT Container tests#17070

Closed
bhagyapathak wants to merge 10 commits into
tomls/base/mainfrom
bhagya/bvt-container-tests
Closed

feat: Enable BVT Container tests#17070
bhagyapathak wants to merge 10 commits into
tomls/base/mainfrom
bhagya/bvt-container-tests

Conversation

@bhagyapathak
Copy link
Copy Markdown
Contributor

@bhagyapathak bhagyapathak commented May 7, 2026

Merge Checklist

All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)

  • The toolchain has been rebuilt successfully (or no changes were made to it)
  • The toolchain/worker package manifests are up-to-date
  • Any updated packages successfully build (or no packages were changed)
  • Packages depending on static components modified in this PR (Golang, *-static subpackages, etc.) have had their Release tag incremented.
  • Package tests (%check section) have been verified with RUN_CHECK=y for existing SPEC files, or added to new SPEC files
  • All package sources are available
  • cgmanifest files are up-to-date and sorted (./cgmanifest.json, ./toolkit/scripts/toolchain/cgmanifest.json, .github/workflows/cgmanifest.json)
  • LICENSE-MAP files are up-to-date (./LICENSES-AND-NOTICES/SPECS/data/licenses.json, ./LICENSES-AND-NOTICES/SPECS/LICENSES-MAP.md, ./LICENSES-AND-NOTICES/SPECS/LICENSE-EXCEPTIONS.PHOTON)
  • All source files have up-to-date hashes in the *.signatures.json files
  • sudo make go-tidy-all and sudo make go-test-coverage pass
  • Documentation has been updated to match any changes to the build system
  • Ready to merge

Summary

This PR ports tests from the legacy CBL-Mariner ContainerBase BVT to the current pytest-based container BVT.

Runtime tests: [base/images/tests/cases/container-base/runtime/test_container_bvt.py]
Static tests: [base/images/tests/cases/container-base/static/test_container.py]

image image
Change Log
  • Enable BVT Container tests for AZL4
Does this affect the toolchain?

NO

Associated issues
  • #xxxx
Links to CVEs
  • NA
Test Methodology

@bhagyapathak
Copy link
Copy Markdown
Contributor Author

bhagyapathak commented May 7, 2026

BVT Container log file -
Uploading bvt-container-test-logs.txt…

@bhagyapathak bhagyapathak changed the title Enable BVT Container tests feat: Enable BVT Container tests May 7, 2026
@bhagyapathak bhagyapathak force-pushed the bhagya/bvt-container-tests branch from 3593d46 to c8e218a Compare May 7, 2026 08:55
@bhagyapathak bhagyapathak force-pushed the bhagya/bvt-container-tests branch from c8e218a to f5edbe7 Compare May 7, 2026 13:51
Comment thread base/images/tests/utils/pytest_plugin.py Outdated
Comment thread base/images/tests/utils/pytest_plugin.py Outdated
Comment thread base/images/tests/conftest.py Outdated
Comment thread base/images/tests/cases/container-base/test_container_static.py Outdated
Comment thread base/images/tests/cases/container-base/test_container_static.py Outdated

@pytest.mark.runtime_container_tests
@pytest.mark.require_capability("container")
def test_bvt_system_footprint(ssh_exec) -> None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a test that's not really asserting properties of the image-under-test but is rather performing measurements. I'm not sure that it fits into this framework.


@pytest.mark.runtime_container_tests
@pytest.mark.require_capability("container")
def test_bvt_logging(ssh_exec) -> None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

assert ok and len(out) > 0, f"hostname command failed or returned empty: {out!r}"
print(f"Hostname: {out}")

# Routing table (informational)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like there's a desire for informational image collection. Does some of it really want to be a snapshot-based test?

Comment thread base/images/tests/utils/container_runtime.py Outdated
Comment thread base/images/tests/utils/container_runtime.py Outdated
@reubeno
Copy link
Copy Markdown
Member

reubeno commented May 11, 2026

Did you consider allowing test cases to provide custom Dockerfiles for more advanced setup? I thought I saw some of the golden container tests, for example, use those.

How would distroless container setup be done?

@bhagyapathak bhagyapathak marked this pull request as ready for review May 12, 2026 18:38
Copilot AI review requested due to automatic review settings May 12, 2026 18:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to enable Container Base BVT (Business/Build Verification) testing for AZL4 images by adding a new runtime (live container) pytest suite alongside existing static/offline image checks.

Changes:

  • Added a new pytest-driven “container-bvt-tests” suite to the container-base image configuration.
  • Introduced podman-based container runtime utilities and pytest fixtures/markers to support executing commands inside a live container.
  • Reorganized/expanded container-base test coverage into static rootfs checks and new runtime BVT checks.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 17 comments.

Show a summary per file
File Description
base/images/tests/utils/pytest_plugin.py Registers an additional pytest marker for package preinstall; plugin marker coverage likely needs expanding for the new runtime suite.
base/images/tests/utils/container_runtime.py New podman orchestration helpers for starting/executing in/destroying test containers.
base/images/tests/conftest.py Adds runtime-container fixtures and a requires_pkg autouse installer hook.
base/images/tests/cases/test_os_release.py Adds an import (currently unused).
base/images/tests/cases/container-base/test_container.py Removes prior container-base test location (moved into static/).
base/images/tests/cases/container-base/static/test_container.py Adds container-base static (offline) checks, including a rootfs size gate.
base/images/tests/cases/container-base/runtime/test_container_bvt.py Adds runtime BVT tests executed via podman exec, including networking/resource/package-management checks.
base/images/images.toml Enables the new container BVT suite for images.container-base and defines the suite configuration.

Comment thread base/images/images.toml Outdated
Comment thread base/images/images.toml Outdated
Comment thread base/images/tests/utils/container_runtime.py Outdated
Comment thread base/images/tests/utils/container_runtime.py Outdated
Comment thread base/images/tests/utils/container_runtime.py Outdated
Comment thread base/images/tests/cases/container-base/runtime/test_container_bvt.py Outdated
Comment thread base/images/tests/cases/container-base/static/test_container.py Outdated
Comment thread base/images/tests/cases/test_os_release.py Outdated
…/ tests with runtime_container_tests + runtime-package-management
@bhagyapathak
Copy link
Copy Markdown
Contributor Author

Did you consider allowing test cases to provide custom Dockerfiles for more advanced setup? I thought I saw some of the golden container tests, for example, use those.

How would distroless container setup be done?

Currently no tests require container creation using Dockerfile hence skipped the implementation in this PR. Do you want to have base code ready to take Dockerfile as input args in this PR itself?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (3)

base/images/tests/conftest.py:248

  • running_container creates a fresh container per test, but it always calls create_container_with_exec(...) without image_ref, which triggers a podman load from the OCI archive each time. For --image-path pointing at a tarball, this can add ~10s per test and make the suite much slower than necessary. Consider adding a session-scoped fixture that resolves/loads the image once and passes image_ref= into create_container_with_exec for each per-test container run.
    logger.info("Creating fresh container for test %s", request.node.name)
    container = create_container_with_exec(
        image_path,
        workdir,
        container_name=None,  # auto-generate a unique name per test
        image_name=image_name,
    )

base/images/tests/cases/container-base/runtime/test_container_bvt.py:454

  • This test hard-depends on external connectivity to http://httpbin.org/get and asserts a ≥90% success rate across 50 fetches. That makes CI runs vulnerable to transient Internet/DNS issues or outbound network policy (flaky failures unrelated to the image). Consider gating this behind an explicit opt-in (env var/marker), using a repo-controlled endpoint, or reducing it to a best-effort informational check (skip/fail only when network is expected to be available).
@pytest.mark.require_capability("container")
@pytest.mark.require_capability("runtime-package-management")
@pytest.mark.requires_pkg("curl")
def test_bvt_sustained_http_fetch(container_exec) -> None:
    """BVT: Sustained external HTTP — 50 sequential fetches, ≥90% success.

    Ports the legacy "Core Networking Test" (50-iteration page fetch).
    """
    iterations = 50
    url = "http://httpbin.org/get"
    # One bash loop is far cheaper than 50 podman exec invocations.
    cmd = (
        f"i=0; ok=0; while [ $i -lt {iterations} ]; do "
        f"curl -s -o /dev/null -w '%{{http_code}}\\n' "
        f"--connect-timeout 5 --max-time 10 {url} | "
        f"grep -q '^2' && ok=$((ok+1)); i=$((i+1)); done; echo $ok"
    )
    ok, out = _cmd_ok(container_exec, cmd, timeout=iterations * 12)
    assert ok, f"Sustained fetch loop failed: {out!r}"
    success = int(out.strip().splitlines()[-1])
    success_rate = success / iterations
    print(f"Sustained HTTP fetch: {success}/{iterations} succeeded ({success_rate:.0%})")
    assert success_rate >= 0.9, \
        f"HTTP success rate {success_rate:.0%} below 90% threshold ({success}/{iterations})"

base/images/tests/conftest.py:266

  • The container_exec fixture docstring says the framework “does not install packages or mutate the container in any way”, but _apply_requires_pkg (autouse) will install packages into the live container for tests marked requires_pkg. Please update the docstring to reflect the new behavior (or narrow the statement to “unless requires_pkg is used”).
    """Execute a shell command in the running container via ``podman exec``.

    Returns a callable ``(command: str, timeout: int = 60) ->
    subprocess.CompletedProcess[str]``. The framework does **not** install
    packages or mutate the container in any way — tests see the image as
    shipped. If a test needs a binary that the image does not ship, it
    must declare the dependency itself (and either skip or fail loudly).
    """

Comment thread base/images/images.toml
Comment thread base/images/tests/conftest.py Outdated
…twork is restricted

Agent-Logs-Url: https://github.com/microsoft/azurelinux/sessions/5a6f8871-2c4d-409b-86ad-2f2607b38e11

Co-authored-by: bhagyapathak <13693535+bhagyapathak@users.noreply.github.com>
@bhagyapathak
Copy link
Copy Markdown
Contributor Author

@copilot apply changes based on the comments in this thread

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Comment thread base/images/tests/utils/pytest_plugin.py Outdated
Comment thread base/images/tests/conftest.py
Comment thread base/images/tests/utils/container_runtime.py Outdated
…g, subprocess timeout

Agent-Logs-Url: https://github.com/microsoft/azurelinux/sessions/66b748da-ab1a-49bd-a6de-a5c86adc4061

Co-authored-by: bhagyapathak <13693535+bhagyapathak@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented May 12, 2026

@copilot apply changes based on the comments in this thread

Applied in commit 9d99f4c:

  1. pytest_plugin.py:121 — Updated runtime_container_tests marker description to clarify it is for -m filtering only; container startup is driven by running_container fixture usage.

  2. conftest.py:251 — Added a session-scoped _container_image_ref fixture that calls _get_image_reference() (which runs podman load) exactly once per test session. running_container now receives the pre-resolved reference and passes it as image_ref= to create_container_with_exec, skipping the podman load for all subsequent tests.

  3. container_runtime.py:44 — Added timeout: int = 300 parameter to _run_container_cmd() so podman load/run/stop/rm operations cannot hang indefinitely in CI.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

base/images/tests/utils/pytest_plugin.py:160

  • require_capability is documented as taking exactly one capability, but the implementation silently accepts 0 or multiple args and only checks marker.args[0]. This can reintroduce incomplete gating if someone accidentally passes multiple args again. Consider validating len(marker.args) == 1 and raising pytest.UsageError (or failing the test) when misused so the mistake is caught immediately.
def pytest_runtest_setup(item: pytest.Item) -> None:
    """Skip tests whose marks are not satisfied."""
    # require_capability: skip if image doesn't have the required capability.
    # Each marker takes exactly one capability name; stack the decorator
    # multiple times to require several capabilities on a single test.
    caps = parse_capabilities(item.config.getoption("--capabilities", default=None))
    for marker in item.iter_markers("require_capability"):
        required = marker.args[0] if marker.args else None
        if required and required not in caps:
            pytest.skip(f"requires capability '{required}' (not in: {sorted(caps)})")

Comment on lines +27 to +32
from utils.container_runtime import (
ContainerExecInstance,
create_container_with_exec,
destroy_exec_container,
_get_image_reference,
)
Comment thread base/images/tests/conftest.py Outdated
image_path: Path, image_type: str, image_name: str | None, workdir: Path,
_container_image_ref: str | None,
request: pytest.FixtureRequest
) -> ContainerExecInstance | None:
Comment on lines +314 to +334
markers = list(request.node.iter_markers("requires_pkg"))
if not markers:
return
if "container_exec" not in request.fixturenames:
pytest.skip("requires_pkg marker is only valid for runtime container tests")

pkgs: list[str] = []
for m in markers:
pkgs.extend(m.args)
if not pkgs:
return

container_exec = request.getfixturevalue("container_exec")
cmd = "dnf install -y " + " ".join(pkgs)
logger.info("requires_pkg: installing %s", pkgs)
result = container_exec(cmd, timeout=300)
if result.returncode != 0:
pytest.skip(
f"requires_pkg: failed to install {pkgs} (rc={result.returncode}): "
f"{result.stderr.strip()[:200]}"
)
Comment on lines +136 to +153
def create_container_with_exec(
image_path: Path,
workdir: Path,
container_name: str | None = None,
image_name: str | None = None,
image_ref: str | None = None,
) -> ContainerExecInstance:
"""Create a running container with podman exec access.

Args:
image_path: Path to image file or image reference
workdir: Working directory (unused but kept for compatibility)
container_name: Container name (auto-generated if None)
image_name: Image name for logging (derived if None)
image_ref: Pre-resolved image reference (skips ``podman load``).
When provided, ``image_path`` is not touched. Use this to
amortize the ~10s tarball load across many container creates.
"""
"Container Size Max Check": walks the extracted rootfs and asserts
the total on-disk size is under AZL_CONTAINER_MAX_SIZE_MB (MiB).
"""
max_mb = int(os.environ.get("AZL_CONTAINER_MAX_SIZE_MB", _DEFAULT_MAX_ROOTFS_MB))
@bhagyapathak
Copy link
Copy Markdown
Contributor Author

@copilot apply changes based on the comments in this thread

…ection, unused params, env validation

Agent-Logs-Url: https://github.com/microsoft/azurelinux/sessions/310e39f7-537e-488a-9b12-f763068ffefc

Co-authored-by: bhagyapathak <13693535+bhagyapathak@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented May 12, 2026

@copilot apply changes based on the comments in this thread

Applied in commit 743b470:

  1. conftest.py:27-32 — Switched to importing the now-public resolve_image_reference (renamed from _get_image_reference in container_runtime.py).

  2. conftest.py:242running_container return type changed from ContainerExecInstance | None to ContainerExecInstance.

  3. conftest.py:314-334_apply_requires_pkg now detects the available package manager in the container (prefers tdnf, falls back to dnf, skips with a clear message if neither is found).

  4. container_runtime.py:136-153workdir parameter removed from create_container_with_exec; image_name is now included in the startup log message.

  5. static/test_container.py:110AZL_CONTAINER_MAX_SIZE_MB parsing is now wrapped in try/except ValueError with a pytest.fail that shows the offending value and expected format.

Copy link
Copy Markdown
Member

@reubeno reubeno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates; I've left comments and questions throughout the code. Happy to meet synchronously to go over it.

I think my biggest questions are around the structure.

In my earlier feedback I was thinking something along these lines:

base/images/tests/
    cases/
        static/
            test_*.py  # shared static tests useful across images; majority of static tests
            <image_name>/
                test_*.py # tests specific to the named image; should be quite minimal
        runtime/
            test_*.py # shared runtime tests useful across images; majority of runtime tests
            <image_name>/
                test_*.py # tests specific to the named image; should be quite minimal

This would allow the static test suite definition to just point to static/ and the runtime one to point to runtime/.

Would be good to get your input here -- and what structure you were going for.

Comment thread base/images/images.toml
"--image-path", "{image-path}",
"--image-name", "{image-name}",
"--capabilities", "{capabilities}",
"-v", "-s"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it makes sense to hard-code -v or -s in the test case, as that's a generic pytest option that the adapter for running pytest-based tests should be able to handle. azldev already does something like that, where it translates its own verbosity options to --log-cli-level. We could refine that, but it doesn't seem like should be test suite specific.

Comment thread base/images/images.toml
[test-suites.container-bvt-tests.pytest]
working-dir = "tests"
install = "pyproject"
test-paths = ["cases/container-base/runtime/test_container_bvt.py"]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just specify the whole runtime/ dir? But also, it's odd that the naming of the test suite implies it should be runnable against any container, but it's got a hard-coded reference to container-base in particular.

@pytest.mark.require_capability("runtime-package-management")
@pytest.mark.requires_pkg("procps-ng", "gawk")
def test_bvt_system_footprint(container_exec) -> None:
"""BVT: Memory, disk, CPU, and process footprint metrics."""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't understand what it means to have a test that doesn't really assert anything meaningful (beyond the ability to run the commands below). This seems more like a metrics-gathering piece of code than a validation test case--do we need a different way to capture metrics?

message = f"BVT log test entry {tag}"

ok, _ = _cmd_ok(container_exec, f"logger -t bvt_test '{message}'", timeout=10)
assert ok, "logger command failed — util-linux may be missing"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem to make sense to say that util-linux may be missing if we're already requiring it via the decorator?


# Find the entry: try journalctl, then /var/log/messages, then /var/log/syslog
found = False
ok, out = _cmd_ok(container_exec, f"journalctl --no-pager --since '1 minute ago' 2>/dev/null | grep '{tag}' || true", timeout=15)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is journalctl even in the base container? We don't run in systemd in it. Was this meant to be a VM test?

assert path.exists(), f"File {path} does not exist"


def test_file_exists_etc_dnf_dnf_conf(rootfs: Path) -> None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good example of a test that could apply to all images that support runtime package management, and not even just containers. I don't think it belongs under container-base.

assert path.exists(), f"License file {path} does not exist"


def test_license_coreutils_single_exists(rootfs: Path) -> None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason that bash and coreutils-single were called out in particular from a license perspective? Or are they spot checks?

return result


def _parse_loaded_images(load_stdout: str) -> list[str]:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't text-scrape command output -- there must be a better programmatic way to get the data we need.

# SPDX-License-Identifier: MIT
"""Container runtime orchestration for dynamic testing.

Provides fixtures and utilities for creating running containers with exec access
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we're not using a higher-level Python library for automating podman? At least something like https://pypi.org/project/podman/? Worthwhile to spend a few mins looking to see if there's something even higher-level that's geared toward testing?

container_exec = request.getfixturevalue("container_exec")

# Detect which package manager is available in this container.
# AZL images may ship tdnf, dnf, or both; prefer tdnf when present.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason to use tdnf if these tests are only intended for 4.0+.

Copy link
Copy Markdown
Collaborator

@christopherco christopherco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retarget to 4.0 branch

@bhagyapathak
Copy link
Copy Markdown
Contributor Author

Closing this PR in favor of #17372

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants